Skip to content

Fix list item creation to preserve list properties atomically#320

Open
jaredpereira wants to merge 11 commits into
mainfrom
claude/wonderful-archimedes-zcz4nw
Open

Fix list item creation to preserve list properties atomically#320
jaredpereira wants to merge 11 commits into
mainfrom
claude/wonderful-archimedes-zcz4nw

Conversation

@jaredpereira

Copy link
Copy Markdown
Contributor

Description

Fixes a race condition when pressing Enter to create a new list item. The issue occurred because list properties (style, checklist status) were being set in separate mutations after the block was created, allowing concurrent operations to see a briefly non-list block and insert plain paragraphs instead.

Changes

components/Blocks/TextBlock/keymap.ts:

  • Use the freshly-read sibling position instead of the stale propsRef snapshot when generating the position for a new list item, preventing collisions with just-created items
  • Move list property queries (block/list-style, block/check-list) earlier in the flow
  • Pass list properties to addBlock mutation instead of setting them separately
  • Improve focus retry logic: instead of a fixed 10ms timeout, retry up to 50 times with exponential backoff until the new block's editor mounts, preventing focus loss

src/replicache/mutations.ts:

  • Add optional list parameter to addBlock mutation to set list properties atomically in the same mutation as block creation
  • Set block/is-list, block/list-style, and block/check-list facts within the same transaction, ensuring the block is never briefly a non-list block

docs/editor-behaviors.md:

  • Added documentation of expected Enter behavior on blank list items (outdent if nested, convert to plain block if at top level)

Why This Matters

Previously, the window between block creation and property assignment allowed racing Enter keypresses to see an incomplete block state, breaking list nesting. Now all list properties are set atomically with block creation.

Checklist

  • Looks good on mobile and desktop
  • Undo works correctly (properties are set in same mutation)
  • Keyboard interactions work (Enter on blank list items, focus retry logic)
  • No build errors

https://claude.ai/code/session_01V4wW9QQpVzduvU5K3BzNGq

claude added 4 commits June 15, 2026 02:39
Start a living doc of expected text editor behaviors. First entry: Enter
on a blank list item outdents one level, and at the top level converts the
item to a normal non-list block while preserving its text type (heading,
blockquote, or plain text).

https://claude.ai/code/session_01V4wW9QQpVzduvU5K3BzNGq
Both the create and outdent paths drove structural mutations off React-prop
snapshots (which lag the Replicache store) and a fixed focus timer, so fast
"type, Enter, type, Enter" or repeated Enter-on-blank produced lost focus,
colliding positions, transiently non-list items, and dropped blocks.

Create path (keymap `enter`):
- Position the new sibling from the freshly-read sibling rather than the
  stale `propsRef.current.position`, so it can't collide with or sort before
  the item just created.
- Create the list item atomically: read inherited list-style/check-list up
  front and write type + is-list + style + checklist in a single `addBlock`
  mutation, closing the window where the new block wasn't yet a list item and
  a follow-up Enter inserted a plain paragraph.
- Focus when the new block's editor actually registers (retry) instead of a
  fixed 10ms timeout that silently dropped focus, sending keystrokes to the
  previous block.

Outdent path (`outdentBlock`):
- Validate the destination anchor before any retraction. Previously a
  stale/missing `after` returned after the block was already retracted and its
  following siblings re-parented under it, orphaning the block and everything
  below it.

https://claude.ai/code/session_01V4wW9QQpVzduvU5K3BzNGq
Drop comments that just narrate what the code shows; keep the non-obvious
why (atomic creation avoids a non-list window, validate-before-retract avoids
orphaning, focusBlock no-ops until mount).

https://claude.ai/code/session_01V4wW9QQpVzduvU5K3BzNGq
@vercel

vercel Bot commented Jun 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
minilink Ready Ready Preview, Comment Jun 16, 2026 5:13am

Request Review

Conflict in components/Blocks/TextBlock/keymap.ts: main moved undo grouping
into the enter command (um.startGroup / asyncRun().finally(endGroup)); kept
that alongside this branch's focus-retry that waits for the new block's editor
to register instead of a fixed 10ms timeout. addBlock list-atomicity and
outdentBlock validate-before-retract auto-merged cleanly.

https://claude.ai/code/session_01V4wW9QQpVzduvU5K3BzNGq
Conflict in keymap.ts focus handling: main's "handle focus on redo" extracted
focusNewBlock and added an undo-redo handler to move the cursor into the new
block on redo. Kept main's redo handling and .finally(endGroup) placement, and
folded this branch's retry-until-the-editor-registers into focusNewBlock so
both the normal and redo focus paths are resilient to the mount race.

https://claude.ai/code/session_01V4wW9QQpVzduvU5K3BzNGq
indent fired retractFact + addLastBlock without awaiting them. main's undo
refactor closes the undo group when the command promise settles (replacing the
old 100ms endGroup timeout), so the un-awaited mutators registered their undo
ops after the group closed — as two separate entries. One undo then reversed
only addLastBlock (retracting the new card/block fact) and orphaned the block;
a second undo restored it.

Await both mutations (as outdent already does) so their undo ops land inside
the group and a single undo restores the block to its prior position.

https://claude.ai/code/session_01V4wW9QQpVzduvU5K3BzNGq
claude added 2 commits June 16, 2026 04:29
Three independent defects surfaced by the subagent investigation:

- retractFact's redo closure omitted ignoreUndo, so redoing any indent/outdent
  re-entered the undo manager — truncating the redo stack and pushing a dangling
  entry that double-referenced then lost the block. Add ignoreUndo (every other
  closure already passes it).

- indent did retract-old-fact + add-new-factID. A mis-grouped undo could apply
  only the add and orphan the block. Reparent the existing card/block fact in a
  single mutation (reuse its factID) so it's one symmetric undo entry that can't
  be half-applied.

- the nested sibling sort in getBlocks had no id tiebreak (the top-level sorts
  do), so equal/colliding positions rendered in unstable order. Add the tiebreak.

https://claude.ai/code/session_01V4wW9QQpVzduvU5K3BzNGq
Grouping: a single shared isGrouping flag was driven by three producers — Tab's
withUndoGroup, the keymap Enter/Backspace groups, and a 200ms text-edit timeout
group in trackUndoRedo — that closed each other's groups and split a command's
mutations across boundaries (orphaning blocks on undo) or merged a Tab into
adjacent typing. Centralize all grouping in the undo manager:
- command groups are depth-counted (nesting-safe) and flush any open text-edit
  group first, so a command never merges with typing;
- text edits coalesce through addGrouped/flushGroup (the manager owns the 200ms
  window), so trackUndoRedo no longer races a per-component timer;
- undo/redo are serialized so rapid keypresses don't re-enter @rocicorp/undo's
  recursive group walk and corrupt the stack.

Focus: indent/outdent change a block's parent but keep its entityID. parent was
a dependency of the ProseMirror mount effect, so every structural edit (and its
undo/redo) destroyed and recreated the EditorView, dropping the caret to <body>.
Drop parent from the dependency array (it's read via propsRef) so the view
survives reparenting and the caret follows the block.

https://claude.ai/code/session_01V4wW9QQpVzduvU5K3BzNGq
addGrouped coalesced every text edit within the 200ms window regardless of
source, so edits to two different blocks within the window merged into one undo
step (only the blur flush saved it). Take a span key (the block/footnote
entityID): consecutive same-key edits coalesce within the window, and a
different key finalizes the open group immediately so distinct spans never
merge — independent of timing or blur. trackUndoRedo threads the editor's
entityID through as the key.

https://claude.ai/code/session_01V4wW9QQpVzduvU5K3BzNGq
Text-edit coalescing was correct, but the group got finalized between
keystrokes, so each character became its own undo step:

- The <pre> onBlur handler called undoManager.flushGroup() on every blur, and
  focus churn during the deferred-updateState re-render fired it mid-run.
  Remove the blur flush (and the now-unused flushGroup); the idle timer, command
  groups, span-key changes, and undo/redo already provide every real boundary.
- COALESCE_MS was 200ms — shorter than a normal inter-keystroke gap (typing
  below ~5 cps), so the timer flushed between keystrokes. Raise to 500ms; the
  timer resets each keystroke, so a run now coalesces until an actual pause.

https://claude.ai/code/session_01V4wW9QQpVzduvU5K3BzNGq
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants